Closed Bug 1375596 Opened 8 years ago Closed 8 years ago

SEGV near null in [@ mozilla::StyleAnimationValue::AddWeighted]

Categories

(Core :: SVG, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: tsmith, Assigned: birtles)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Attachments

(2 files)

Attached file test_case.html
==14379==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000020 (pc 0x7f832a43546b bp 0x7fffc3779390 sp 0x7fffc3778fe0 T0) ==14379==The signal is caused by a READ memory access. ==14379==Hint: address points to the zero page. #0 0x7f832a43546a in GetUnit src/obj-firefox/dist/include/mozilla/StyleAnimationValue.h:356:12 #1 0x7f832a43546a in mozilla::StyleAnimationValue::AddWeighted(nsCSSPropertyID, double, mozilla::StyleAnimationValue const&, double, mozilla::StyleAnimationValue const&, mozilla::StyleAnimationValue&) src/layout/style/StyleAnimationValue.cpp:2869 #2 0x7f8329b271c7 in Add src/obj-firefox/dist/include/mozilla/StyleAnimationValue.h:62:12 #3 0x7f8329b271c7 in AddOrAccumulate(nsSMILValue&, nsSMILValue const&, mozilla::dom::CompositeOperation, unsigned long) src/dom/smil/nsSMILCSSValueType.cpp:413 #4 0x7f8329b2664b in nsSMILCSSValueType::SandwichAdd(nsSMILValue&, nsSMILValue const&) const src/dom/smil/nsSMILCSSValueType.cpp:422:10 #5 0x7f8329b1dbc7 in nsSMILAnimationFunction::ComposeResult(nsISMILAttr const&, nsSMILValue&) src/dom/smil/nsSMILAnimationFunction.cpp:271:22 #6 0x7f8329b1a7ce in nsSMILCompositor::ComposeAttribute(bool&) src/dom/smil/nsSMILCompositor.cpp:108:29 #7 0x7f8329b17e6a in nsSMILAnimationController::DoSample(bool) src/dom/smil/nsSMILAnimationController.cpp:455:17 #8 0x7f832a786bbb in Resample src/obj-firefox/dist/include/nsSMILAnimationController.h:74:21 #9 0x7f832a786bbb in FlushResampleRequests src/obj-firefox/dist/include/nsSMILAnimationController.h:90 #10 0x7f832a786bbb in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4165 #11 0x7f832689e1f1 in FlushPendingNotifications src/obj-firefox/dist/include/nsIPresShell.h:560:5 #12 0x7f832689e1f1 in nsDocument::FlushPendingNotifications(mozilla::FlushType) src/dom/base/nsDocument.cpp:8076 #13 0x7f8325842fdb in nsDocLoader::DocLoaderIsEmpty(bool) src/uriloader/base/nsDocLoader.cpp:702:14 #14 0x7f8325845302 in nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:631:5 #15 0x7f832584602c in non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult) src/uriloader/base/nsDocLoader.cpp:487:14 #16 0x7f8323f95753 in mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult) src/netwerk/base/nsLoadGroup.cpp:629:28 #17 0x7f83268a505b in nsDocument::DoUnblockOnload() src/dom/base/nsDocument.cpp:8929:18 #18 0x7f83268a4c43 in nsDocument::UnblockOnload(bool) src/dom/base/nsDocument.cpp:8855:9 #19 0x7f832687c56d in nsDocument::DispatchContentLoadedEvents() src/dom/base/nsDocument.cpp:5348:3 #20 0x7f8326947ed2 in applyImpl<nsDocument, void (nsDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1084:12 #21 0x7f8326947ed2 in apply<nsDocument, void (nsDocument::*)()> src/obj-firefox/dist/include/nsThreadUtils.h:1090 #22 0x7f8326947ed2 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() src/obj-firefox/dist/include/nsThreadUtils.h:1133 #23 0x7f8323dfb798 in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1416:14 #24 0x7f8323e08678 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:472:10 #25 0x7f8324bb5221 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:96:21 #26 0x7f8324b133a0 in RunInternal src/ipc/chromium/src/base/message_loop.cc:318:10 #27 0x7f8324b133a0 in RunHandler src/ipc/chromium/src/base/message_loop.cc:311 #28 0x7f8324b133a0 in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:291 #29 0x7f832a0699ef in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27 #30 0x7f832e0e6431 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:283:30 #31 0x7f832e2b6674 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4569:22 #32 0x7f832e2b81e0 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4749:8 #33 0x7f832e2b9531 in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4844:21 #34 0x4eb613 in do_main src/browser/app/nsBrowserApp.cpp:237:22 #35 0x4eb613 in main src/browser/app/nsBrowserApp.cpp:310 #36 0x7f834099182f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291 #37 0x41d168 in _start (browsers/m-c-1497454412-asan-opt/firefox+0x41d168) AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV src/obj-firefox/dist/include/mozilla/StyleAnimationValue.h:356:12 in GetUnit
Flags: in-testsuite?
The test case is SMIL animation.
Component: DOM: Animation → SVG
Likely a recent regression from all the refactoring we've done of that code for stylo.
Priority: -- → P1
I'm still waiting for a build to finish to debug this but the following changeset from bug 1358966 looks suspicious since we dereference valueToAddWrapper without any guarantee that it is not null (prior to that changeset we dereferenced valueToAdd which we do null-check FinalizeStyleAnimationValues): https://hg.mozilla.org/mozilla-central/rev/4d87f2bf4b10369af0dd83a2ef962a23299ee8d9
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
Comment on attachment 8887358 [details] Bug 1375596 - Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper; https://reviewboard.mozilla.org/r/158190/#review163462
Attachment #8887358 - Flags: review?(hikezoe) → review+
Pushed by bbirtles@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/86878427cd44 Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper; r=hiro
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Please request Beta approval on this when you get a chance.
Flags: needinfo?(bbirtles)
Comment on attachment 8887358 [details] Bug 1375596 - Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper; Approval Request Comment [Feature/Bug causing the regression]: bug 1358966 [User impact if declined]: Content process crash when opening offending SVG content [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No (has automated test) [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Simply makes us dereference a pointer that is known to be non-null as opposed to one that is not guaranteed to be non-null [String changes made/needed]: None
Flags: needinfo?(bbirtles)
Attachment #8887358 - Flags: approval-mozilla-beta?
Comment on attachment 8887358 [details] Bug 1375596 - Use valueToAdd in AddAccumulateOrValue, not valueToAddWrapper; fix nullptr deref crash, beta55+
Attachment #8887358 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Brian Birtles (:birtles) from comment #9) > [Is this code covered by automated tests?]: Yes > [Has the fix been verified in Nightly?]: Yes > [Needs manual test from QE? If yes, steps to reproduce]: No (has automated > test) Setting qe-verify- based on Brian's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: